Skip to content

Add ARIA14 reference to F86 - #5331

Open
patrickhlauke wants to merge 2 commits into
mainfrom
patrickhlauke-issue3791
Open

Add ARIA14 reference to F86#5331
patrickhlauke wants to merge 2 commits into
mainfrom
patrickhlauke-issue3791

Conversation

@patrickhlauke

Copy link
Copy Markdown
Member

Closes #3791

@github-project-automation github-project-automation Bot moved this to To do in WCAG 2.x Aug 26, 2026
@patrickhlauke patrickhlauke moved this from To do to Drafted in WCAG 2.x Aug 26, 2026
@patrickhlauke patrickhlauke self-assigned this Aug 26, 2026
@netlify

netlify Bot commented Aug 26, 2026

Copy link
Copy Markdown

Deploy Preview for wcag2 ready!

Name Link
🔨 Latest commit fabd662
🔍 Latest deploy log https://app.netlify.com/projects/wcag2/deploys/6a9ddb9c996d1b0008f3a5af
😎 Deploy Preview https://deploy-preview-5331--wcag2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@patrickhlauke patrickhlauke moved this from Drafted to Ready for approval in WCAG 2.x Aug 28, 2026
tobie pushed a commit to specinfra/pr-preview that referenced this pull request Sep 7, 2026
The hook handler logged bare words ("comment", "issue", "skipping
auto-generated changes") and dumped the whole payload for anything
unrecognised, while pull_request actions it doesn't build on (closed,
labeled, ...) fell through the switch without a trace. Every branch now
logs the event type, the action, the PR or issue it concerned and why it
was ignored or skipped:

    Ignoring pull_request "closed" event on w3c/wcag/5331: not an action we build on
    Ignoring pull_request "edited" event on w3c/wcag/5331: triggered by our own update
    Ignoring issue_comment "created" event on w3c/wcag#5331: only pull_request events are handled
    Ignoring "ping" event: only pull_request events are handled (payload keys: zen, hook)
    Skipping pull_request "synchronize" event on w3c/wcag/5331: already processing

The actions that trigger a build are a named list rather than switch
fall-through cases. Covered by server tests; the writeup is updated.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012DwdJGN9eH36jA1QZez1F3
tobie added a commit to specinfra/pr-preview that referenced this pull request Sep 7, 2026
* Tighten error handling and logging around job processing

Follow-up to #200. The dismissal reasons made the log say why a job was
dropped; this pass removes the duplication and dead paths around them and
fixes a real bug they had been hiding.

Requeue was broken. updateBody() throws an aborted error carrying
`requeue: true` when commits land during a build, but handlePullRequest()
catches every error, and since the "Modernize codebase" refactor (#167)
the result it returns no longer carried the flag. processQueue() only ever
saw `result.requeue === undefined`, so a PR that got new commits mid-build
was logged as "requeued" and then never rebuilt until the next webhook.
The result now carries `requeue`, and processQueue() puts the job back
once it is released from the running set. Covered by tests.

One source of truth for what gets reported on the PR. shouldReportError()
and reportSkipReason() were two hand-mirrored lists; the former is now the
negation of the latter, as needsUpdate() already was of updateSkipReason().
isInternalError() went with it. A PR that never loaded has nothing to post
to, so that check now comes before the forced-update override rather than
letting a forced job attempt a render with no payload.

handlePullRequest() always resolves. Building the PR object and rendering
the error report both happened outside a try, so a bad job id or a render
failure escaped to processQueue()'s catch and was logged bare, without
the job id. Both are inside now, and reportError() folds a failure to post
into the result as `errorReportingError` (the old name,
`errorRenderingErrorMsg`, described an error that could not reach it).
processQueue() keeps a catch for a failing result handler, so one bad
result cannot stall the queue.

Marking a dismissal is one call. `dismiss(err, flag, reason)` in
error-utils replaces the three copies of "set a flag, set a reason, throw"
in config.js, pr.js and controller.js, and rejects unknown flags.

Logger. One logger instance is created in index.js and handed to the
controller and the app, instead of the app building a second one and the
controller writing to console directly, and the nextTick deferral is gone
so lines come out in program order. logError() is the one place that
formats an error (headline, attached data, stack when
DISPLAY_STACK_TRACES is set) and is used by the result logger, the /config
route, the queue loop and the startup queue; the /config route used to
print the stack unconditionally and the headline twice. logResult() no
longer dumps the whole result object after every job, which had grown to
include the full rendered PR body; the summary line now says what
happened instead:

    w3c/wcag/5331: synchronize (updated)
    w3c/wcag/5331: edited (no update: rendered body is already up to date)
    w3c/wcag/5331: opened (not a live run, would have updated)

Startup queue. Validation lives in parseStartupQueue(), which logs one
reason and returns null; processStartupQueue() just queues, logs the
job ids on one line, and awaits the drain so the catch in index.js
actually covers processing.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012DwdJGN9eH36jA1QZez1F3

* Document error handling and its cases

Adds docs/error-handling.md: the three outcomes a job can have and where
each leaves a trace, the result object, how dismissals are flagged, the
order in which the controller decides whether an error is reported on the
PR, what the PR report and the log show, and one section per case with
the exact log line it produces. Linked from CONTRIBUTING.md.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012DwdJGN9eH36jA1QZez1F3

* Say what each webhook delivery was and what became of it

The hook handler logged bare words ("comment", "issue", "skipping
auto-generated changes") and dumped the whole payload for anything
unrecognised, while pull_request actions it doesn't build on (closed,
labeled, ...) fell through the switch without a trace. Every branch now
logs the event type, the action, the PR or issue it concerned and why it
was ignored or skipped:

    Ignoring pull_request "closed" event on w3c/wcag/5331: not an action we build on
    Ignoring pull_request "edited" event on w3c/wcag/5331: triggered by our own update
    Ignoring issue_comment "created" event on w3c/wcag#5331: only pull_request events are handled
    Ignoring "ping" event: only pull_request events are handled (payload keys: zen, hook)
    Skipping pull_request "synchronize" event on w3c/wcag/5331: already processing

The actions that trigger a build are a named list rather than switch
fall-through cases. Covered by server tests; the writeup is updated.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012DwdJGN9eH36jA1QZez1F3

* Name jobs by their GitHub URL in the log

Every log line that identified a job by its owner/repo/number id now
prints https://github.com/owner/repo/pull/number instead, so a line can
be pasted straight into a browser. Webhook jobs already carried the URL;
queueJob() now derives it from the id for startup-queue jobs, and the
ignored-event lines in the hook handler use the payload's html_url.
Issue events point at the issue.

The writeup's examples use a placeholder org/repo rather than a real one.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012DwdJGN9eH36jA1QZez1F3

* Log ignored and skipped events in the same shape as results

Processed jobs log as "<url>: <action> (<outcome>)". The webhook's
ignored and skipped deliveries, the startup queue's duplicate skips and
the line logged when a job is picked up now use the same shape, so every
per-job line in the log reads the same way and greps the same way:

    https://github.com/org/repo/pull/42: closed (ignored: not an action we build on)
    https://github.com/org/repo/pull/42: synchronize (skipped: already processing)
    https://github.com/org/repo/pull/42: starting (currently running: ...)
    https://github.com/org/repo/pull/42: startup-queue (skipped: already queued)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012DwdJGN9eH36jA1QZez1F3

* Put the status before its detail in per-job log lines

Per-job lines now read "<url> (<action>): <status> (<detail>)", so the
job comes first, then one word says what happened, then the detail:

    https://github.com/org/repo/pull/42 (synchronize): updated
    https://github.com/org/repo/pull/42 (edited): no update (rendered body is already up to date)
    https://github.com/org/repo/pull/42 (opened): dismissed (no .pr-preview.json, repo hasn't opted into previews)
    https://github.com/org/repo/pull/42 (synchronize): failed (Error: 500 Internal Server Error)
    https://github.com/org/repo/pull/42 (closed): ignored (not an action we build on)
    https://github.com/org/repo/pull/42 (synchronize): skipped (already processing)

The logger builds every result line through one jobLine() helper; the
webhook and startup queue follow the same shape.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012DwdJGN9eH36jA1QZez1F3

---------

Co-authored-by: Claude <noreply@anthropic.com>
@patrickhlauke patrickhlauke moved this from Ready for approval to Sent for WG approval in WCAG 2.x Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Sent for WG approval

Development

Successfully merging this pull request may close these issues.

Technique F86: Add related technique ARIA14

2 participants